-
Notifications
You must be signed in to change notification settings - Fork 441
Fix #3177: Update pre-populated search engine list #3189
Conversation
This also replaces Yahoo with Ecosia on search onboarding screen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look great to me! We'll keep Yahoo as an available engine (not in onboarding of course) until we can support having it migrated off (maybe after merging OpenSearch code)
@bsclifton yes, i tried to move yahoo out, but found no simple way to do it, we will revisit it later |
case google, bing, duckduckgo, yandex, qwant, startpage, yahoo | ||
case google, bing, duckduckgo, yandex, qwant, startpage, yahoo, ecosia | ||
|
||
var excludedFromOnboarding: Bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This property is temporary right until we remove the yahoo engine totally ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can keep it, if we add a lot of predefined search engines in the future we can choose which of them to show during onboarding, to avoid having a long list of search engines
This also replaces Yahoo with Ecosia on search onboarding screen.
Summary of Changes
This pull request fixes #3177
Submitter Checklist:
NSLocalizableString()
Test Plan:
Screenshots:
Reviewer Checklist:
QA/(Yes|No)
release-notes/(include|exclude)
bug
/enhancement